-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 26490 remove replacebr #606
Issue 26490 remove replacebr #606
Conversation
Please add test strings in Tests step for easier test. Example: #573. |
Add some more test cases with no line break, multiple line breaks, with/without space
|
@0xmiroslav Do I need to add those cases to the unit testing or to the test strings in the PR description? |
Both |
@0xmiroslav can you please do the review checklist on this one? Thanks! |
@0xmiroslav After this PR is merged, do I need to create another PR in the App to update the expensify-common package version? What will be measured as the days for the PR merged - this PR or another PR for App? |
Just to confirm that this is not bug.
In edit mode:
|
@0xmiroslav Yes, we can see the same result in the current staging environment. |
Screenshots/VideosAndroid: Nativeandroid.movAndroid: mWeb Chromemchrome.moviOS: Nativeios.moviOS: mWeb Safarimsafari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@srikarparsi all yours
@danieldoglas Is there anything I need to do to merge this PR? |
@danieldoglas Can you please update the version of |
@nikosamofa You just need to follow this: https://github.com/Expensify/expensify-common?tab=readme-ov-file#deploying-a-change-expensify-only on the app repo! |
This PR:
replacebr
rule option insideExpensiMark.rules
Fixed Issues
$ Expensify/App#26490
$ Expensify/App#16982
Tests
Unit testing
Testing in the
expensify-app
expensify-app
, opennode_modules/expensify-common/lib/ExpensiMark.js
filereplacebr
ruleTest Strings
Screen.Recording.2023-10-14.at.11.24.25.AM.mov
Screen.Recording.2023-10-14.at.11.25.23.AM.mov
Screen.Recording.2023-11-15.at.9.59.11.AM.mov
Screen.Recording.2023-11-15.at.10.00.14.AM.mov
Screen.Recording.2023-11-15.at.10.00.58.AM.mov
QA
Same as test